Allow evm to call native modules through logs #417

yihuang · 2021-08-09T10:09:37Z

Closes #416

Description

Add EvmHooks, currently only single api PostTxProcessing.
implements a BankSendHook which can send native coins, but not enabled in app by default.

    function test() public {
        emit __CosmosNativeBankSend(msg.sender, 10, "aphoton");
    }

For contributor use:

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

For admin use:

Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
Reviewers assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

codecov · 2021-08-09T10:12:51Z

Codecov Report

Merging #417 (0fc6b8f) into main (b76d024) will increase coverage by 0.12%.
The diff coverage is 72.41%.

@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
+ Coverage   50.41%   50.53%   +0.12%     
==========================================
  Files          57       58       +1     
  Lines        5465     5495      +30     
==========================================
+ Hits         2755     2777      +22     
- Misses       2594     2600       +6     
- Partials      116      118       +2

Impacted Files	Coverage Δ
x/evm/types/errors.go	`100.00% <ø> (ø)`
x/evm/keeper/state_transition.go	`73.12% <45.45%> (-1.16%)`	⬇️
x/evm/keeper/keeper.go	`66.96% <80.00%> (+0.60%)`	⬆️
x/evm/keeper/hooks.go	`100.00% <100.00%> (ø)`

fedekunze · 2021-08-09T21:18:20Z

@yihuang let's create an ADR for this that leverages Hooks. See the governance hooks for reference

netlify · 2021-08-11T16:44:29Z

✔️ Deploy Preview for stoic-ritchie-0f4a47 ready!

🔨 Explore the source changes: 87130edb959b8f1f469814c44b70c30a53b79b3d

🔍 Inspect the deploy log: https://app.netlify.com/sites/stoic-ritchie-0f4a47/deploys/6113fe699f783100085b611c

😎 Browse the preview: https://deploy-preview-417--stoic-ritchie-0f4a47.netlify.app

orijbot · 2021-08-30T08:24:19Z

Visit https://dashboard.github.orijtech.com?back=0&pr=417&remote=true&repo=yihuang%2Fethermint to see benchmark details.

x/evm/types/expected_keepers.go

x/evm/types/hooks.go

x/evm/keeper/keeper_test.go

x/evm/keeper/state_transition.go

Closes #416 comment add txHash parameter review suggestions add hooks test

fedekunze

Final pass. Great work! Can you also add a new Features entry on the Changelog? 🙏

x/evm/keeper/hooks.go

x/evm/keeper/hooks_test.go

x/evm/keeper/keeper.go

x/evm/keeper/state_transition.go

fedekunze · 2021-09-02T07:42:41Z

x/evm/types/interfaces.go

+
+// EvmHooks event hooks for evm tx processing
+type EvmHooks interface {
+	// Must be called after tx is processed, if failed, the whole evm transaction is reverted.


Add the warning that we discussed in the Negative consequences section of the ADR

Actually, that negative consequence is about the use case of utilizing the hook to process logs, not about the hook interface in general.

I added the use case context to consequences in the ADR.

Co-authored-by: Federico Kunze Küllmer <[email protected]>

…-processing

yihuang · 2021-09-02T09:42:28Z

Final pass. Great work! Can you also add a new Features entry on the Changelog? 🙏

changelog added.

fedekunze

ACK 💯

CHANGELOG.md

yihuang requested review from fedekunze and khoslaventures as code owners August 9, 2021 10:09

yihuang marked this pull request as draft August 9, 2021 10:09

yihuang force-pushed the log-post-processing branch from 3c1652c to b7cfc57 Compare August 11, 2021 04:20

yihuang marked this pull request as ready for review August 11, 2021 04:20

yihuang force-pushed the log-post-processing branch from b7cfc57 to fef8a69 Compare August 11, 2021 04:25

yihuang mentioned this pull request Aug 11, 2021

Add adr-002 evm hooks proposal #429

Merged

11 tasks

yihuang force-pushed the log-post-processing branch from da83805 to e6b5d10 Compare August 11, 2021 06:19

yihuang mentioned this pull request Aug 30, 2021

Problem: evm contract can't call native module message crypto-org-chain/cronos#29

Closed

yihuang force-pushed the log-post-processing branch from 87130ed to ff32ae9 Compare August 30, 2021 08:24

fedekunze suggested changes Sep 1, 2021

View reviewed changes

Allow evm to call native modules through logs

9f8f0dd

Closes #416 comment add txHash parameter review suggestions add hooks test

yihuang force-pushed the log-post-processing branch from ff32ae9 to 9f8f0dd Compare September 1, 2021 15:19

yihuang requested a review from fedekunze September 1, 2021 15:21

fedekunze reviewed Sep 2, 2021

View reviewed changes

yihuang and others added 8 commits September 2, 2021 15:47

Update x/evm/keeper/hooks.go

2535f8f

Co-authored-by: Federico Kunze Küllmer <[email protected]>

Update x/evm/keeper/hooks_test.go

e30c119

Co-authored-by: Federico Kunze Küllmer <[email protected]>

Update x/evm/keeper/keeper.go

f3bd48d

Co-authored-by: Federico Kunze Küllmer <[email protected]>

Update x/evm/keeper/keeper.go

7fc8f6d

Co-authored-by: Federico Kunze Küllmer <[email protected]>

Update x/evm/keeper/keeper.go

0868f31

Co-authored-by: Federico Kunze Küllmer <[email protected]>

use table tests

fdff2de

update adr comment

0fef9a6

update adr

a7209ab

yihuang requested a review from fedekunze September 2, 2021 09:27

yihuang added 2 commits September 2, 2021 17:27

Merge branch 'main' into log-post-processing

123e286

changelog

990c562

Merge remote-tracking branch 'fork/log-post-processing' into log-post…

d8d1155

…-processing

fedekunze approved these changes Sep 2, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

6bc1749

fedekunze enabled auto-merge (squash) September 2, 2021 12:21

Merge branch 'main' into log-post-processing

0fc6b8f

fedekunze added the automerge Automatically merge PR once all prerequisites pass. label Sep 2, 2021

fedekunze merged commit 089afe4 into evmos:main Sep 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow evm to call native modules through logs #417

Allow evm to call native modules through logs #417

yihuang commented Aug 9, 2021 •

edited

Loading

codecov bot commented Aug 9, 2021 •

edited

Loading

fedekunze commented Aug 9, 2021

netlify bot commented Aug 11, 2021 •

edited

Loading

orijbot commented Aug 30, 2021

fedekunze left a comment

fedekunze Sep 2, 2021

yihuang Sep 2, 2021

yihuang Sep 2, 2021 •

edited

Loading

yihuang commented Sep 2, 2021

fedekunze left a comment

Allow evm to call native modules through logs #417

Allow evm to call native modules through logs #417

Conversation

yihuang commented Aug 9, 2021 • edited Loading

Description

codecov bot commented Aug 9, 2021 • edited Loading

Codecov Report

fedekunze commented Aug 9, 2021

netlify bot commented Aug 11, 2021 • edited Loading

orijbot commented Aug 30, 2021

fedekunze left a comment

Choose a reason for hiding this comment

fedekunze Sep 2, 2021

Choose a reason for hiding this comment

yihuang Sep 2, 2021

Choose a reason for hiding this comment

yihuang Sep 2, 2021 • edited Loading

Choose a reason for hiding this comment

yihuang commented Sep 2, 2021

fedekunze left a comment

Choose a reason for hiding this comment

yihuang commented Aug 9, 2021 •

edited

Loading

codecov bot commented Aug 9, 2021 •

edited

Loading

netlify bot commented Aug 11, 2021 •

edited

Loading

yihuang Sep 2, 2021 •

edited

Loading